Remove SensorMetricSample in favour of MetricSample
#346
Conversation
This commit removes the `SensorMetricSample` message in favour of using `MetricSample` for sensors. The `SensorMetricSample` message was a subset of the `MetricSample` message. From a design perspective focused on consistency, flexibility, and minimizing API surface area while handling variations via optional fields (which is idiomatic in Protobuf3), there's a strong argument that `SensorMetricSample` is redundant and potentially confusing, and that having only `MetricSample` is a cleaner design. Signed-off-by: Tiyash Basu <[email protected]>
github-actions
bot
added
part:docs
There was a problem hiding this comment.
Pull Request Overview
This PR removes the SensorMetricSample message in favor of using the MetricSample message for sensor metrics to improve consistency and reduce API surface area.
- Replaces the SensorMetricSample type with MetricSample in the SensorData message.
- Updates inline comments and release notes to reflect the change.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| proto/frequenz/api/common/v1/microgrid/sensors/sensors.proto | Updates SensorData to use MetricSample; inline examples in comments updated. |
| RELEASE_NOTES.md | Adds an entry to note the removal of SensorMetricSample. |
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
Shouldn't this remove SensorMetric too?
Done. |
Signed-off-by: Tiyash Basu <[email protected]>
tiyash-basu-frequenz
force-pushed
the
339_sensor_metric_sample
branch
from
8c0e873 to
3567a48
Compare
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
I also wonder if it is worth clarifying that for sensors bounds will always be empty in the SensorData message, but approving as it is something very minor.
I will remember to add it in the next PR. |
2 participants
This commit removes the
SensorMetricSamplemessage in favour of usingMetricSamplefor sensors. TheSensorMetricSamplemessage was a subset of theMetricSamplemessage.From a design perspective focused on consistency, flexibility, and minimizing API surface area while handling variations via optional fields (which is idiomatic in Protobuf3), there's a strong argument that
SensorMetricSampleis redundant and potentially confusing, and that having onlyMetricSampleis a cleaner design.closes #339